Refine the error message of monitor port conflict #966
Conversation
Signed-off-by: JaySon-Huang <tshent@qq.com>
ti-chi-bot
added
the
size/M
Codecov Report
@@ Coverage Diff @@
## master #966 +/- ##
==========================================
- Coverage 55.78% 51.71% -4.08%
==========================================
Files 263 263
Lines 19522 19529 +7
==========================================
- Hits 10891 10100 -791
- Misses 6903 7798 +895
+ Partials 1728 1631 -97
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
pkg/cluster/spec/validate.go
Outdated
| port: port, | ||
| clusterName: name, | ||
| componentName: inst.ComponentName(), | ||
| host: inst.GetHost(), |
There was a problem hiding this comment.
Um, I didn't quite get the point of adding a new field, we could use inst.GetHost() to get the value anytime.
There was a problem hiding this comment.
I try to make the message contains in Entry clear and eliminate the ambiguity. If there exist fields port, componentName along with inst, should I get the port from inst.GetPort() or get the name from inst.ComponentName()?
However, keeping inst instead of adding host is more flexible.
I can keep inst instead of adding host if you insist.
There was a problem hiding this comment.
Yes I would still suggest using inst as it carries more information.
Signed-off-by: JaySon-Huang <tshent@qq.com>
|
/merge |
ti-chi-bot
added
the
status/can-merge
|
Can merge label has been added. Git tree hash: 9fd504c
|
AstroProfundis
added
the
category/monitoring
Signed-off-by: JaySon-Huang tshent@qq.com
What problem does this PR solve?
Fix #965. Refine the error message of monitor port conflict.
What is changed and how it works?
Use
spec.RoleMonitoras the component name instead of the instance name.Check List
Tests
Code changes
Side effects
Related changes
Release notes: